Conversation
…ect in LoadPluginTypesFromDir Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes wfctl validate --plugin-dir falsely reporting “unknown type” for external plugins whose plugin.json uses the registry-style nested capabilities object by loading types from both legacy flat fields and the nested format.
Changes:
- Extend
schema.LoadPluginTypesFromDirto register module/step/trigger/workflow handler types fromcapabilities.*in addition to top-level fields. - Add unit test coverage for the nested
capabilitiesmanifest format. - Add an end-to-end
wfctl validate --plugin-dirtest covering the nestedcapabilitiesformat.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
schema/schema.go |
Adds support for reading types from a nested capabilities object in plugin.json during validation. |
schema/schema_test.go |
Adds a unit test validating type registration from the nested capabilities format. |
cmd/wfctl/main_test.go |
Adds an integration test verifying wfctl validate --plugin-dir succeeds with nested capabilities manifests. |
| type pluginManifestTypes struct { | ||
| ModuleTypes []string `json:"moduleTypes"` | ||
| StepTypes []string `json:"stepTypes"` | ||
| TriggerTypes []string `json:"triggerTypes"` | ||
| WorkflowTypes []string `json:"workflowTypes"` | ||
| ModuleTypes []string `json:"moduleTypes"` | ||
| StepTypes []string `json:"stepTypes"` | ||
| TriggerTypes []string `json:"triggerTypes"` | ||
| WorkflowTypes []string `json:"workflowTypes"` | ||
| Capabilities *pluginManifestCapabilities `json:"capabilities,omitempty"` | ||
| } |
There was a problem hiding this comment.
LoadPluginTypesFromDir unmarshals plugin.json into pluginManifestTypes. With the new Capabilities *pluginManifestCapabilities field, unmarshalling will fail (and the manifest will be skipped entirely) if a plugin.json uses the engine/internal manifest format where capabilities is a JSON array (see plugin.PluginManifest.Capabilities []CapabilityDecl). That would prevent registering the otherwise-valid top-level moduleTypes/stepTypes/etc. Consider making Capabilities a json.RawMessage (or custom UnmarshalJSON) and only decoding it as an object when it’s actually an object, ignoring the array form.
| // pluginManifestCapabilities holds the nested capabilities object used in the | ||
| // v0.3.0+ external plugin.json format (e.g. from wfctl plugin install). | ||
| type pluginManifestCapabilities struct { | ||
| ModuleTypes []string `json:"moduleTypes"` | ||
| StepTypes []string `json:"stepTypes"` | ||
| TriggerTypes []string `json:"triggerTypes"` | ||
| WorkflowHandlers []string `json:"workflowHandlers"` | ||
| } |
There was a problem hiding this comment.
The comment says the nested capabilities object format is used by external plugin.json “(e.g. from wfctl plugin install)”, but wfctl plugin install currently writes a flattened plugin.json with top-level moduleTypes/stepTypes/triggerTypes (see writeInstalledManifest in cmd/wfctl/plugin_install.go). Suggest updating this comment to reflect that this nested format corresponds to the registry manifest shape (or older installs), not the current installer output.
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
|
@copilot apply changes based on the comments in this thread |
…ject formats; update comments Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Applied all three review comments in commit 30077e5:
|
wfctl validate --plugin-dirfailed to recognize module/step/trigger types in external plugins whoseplugin.jsonuses the v0.3.0+ nestedcapabilitiesobject format, causing false "unknown type" validation errors.Root Cause
LoadPluginTypesFromDirinschema/schema.goonly read top-levelmoduleTypes/stepTypes/triggerTypesfields. The v0.3.0+ registry manifest format nests these under acapabilitiesobject:{ "capabilities": { "moduleTypes": ["authz.casbin"], "stepTypes": ["step.authz_check_casbin"], "triggerTypes": [] } }Changes
schema/schema.go: AddedpluginManifestCapabilitiesstruct for the nested format; added it as an optionalCapabilitiesfield onpluginManifestTypes; updatedLoadPluginTypesFromDirto register types from both the flat (legacy) and nestedcapabilitiesformats.schema/schema_test.go: AddedTestLoadPluginTypesFromDir_CapabilitiesFormatcovering the nested format.cmd/wfctl/main_test.go: AddedTestRunValidatePluginDirCapabilitiesend-to-end integration test verifyingwfctl validate --plugin-dirpasses for configs using types declared via thecapabilitiesobject.Original prompt
💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.